fix(pm): preserve message WebView state across recreation (#691)#811
fix(pm): preserve message WebView state across recreation (#691)#811jim-daf wants to merge 1 commit intoAwful:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the PM detail view showing a blank message body after returning from Settings or other fragment recreation events by persisting/restoring the embedded AwfulWebView state and avoiding an unnecessary refetch when a snapshot is available.
Changes:
- Save the message
AwfulWebViewstate inonSaveInstanceState. - Restore the saved
AwfulWebViewstate inonCreateView. - Skip
syncPM()when restoring from saved state.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Issue #691: persist the message WebView so re-entering the | ||
| // fragment after the settings activity does not redraw blank. | ||
| if (messageWebView != null) { | ||
| messageWebView.saveState(outState); | ||
| } |
There was a problem hiding this comment.
WebView#saveState writes keys directly into the provided Bundle; to avoid accidental key collisions with fragment/view state and to make it possible to detect whether anything was actually saved, consider saving into a dedicated nested Bundle (e.g., outState.putBundle(...)) and/or tracking saveState’s return value.
| messageWebView.setContent(AwfulHtmlPage.getContainerHtml(mPrefs, null, false)); | ||
|
|
||
| // Issue #691: restore the saved WebView snapshot when the | ||
| // fragment is recreated so the message body does not come back | ||
| // blank after popping back from the settings activity. | ||
| if (aSavedState != null) { | ||
| messageWebView.restoreState(aSavedState); |
There was a problem hiding this comment.
restoreState is being called after setContent(...) has already loaded the container HTML. Per WebView lifecycle expectations, restore is most reliable before any load; consider restoring first (or skipping setContent entirely when restoring) to avoid overwriting/restoring inconsistently or causing an extra blank load/flicker.
| messageWebView.setContent(AwfulHtmlPage.getContainerHtml(mPrefs, null, false)); | |
| // Issue #691: restore the saved WebView snapshot when the | |
| // fragment is recreated so the message body does not come back | |
| // blank after popping back from the settings activity. | |
| if (aSavedState != null) { | |
| messageWebView.restoreState(aSavedState); | |
| // Issue #691: restore the saved WebView snapshot when the | |
| // fragment is recreated so the message body does not come back | |
| // blank after popping back from the settings activity. | |
| if (aSavedState != null) { | |
| messageWebView.restoreState(aSavedState); | |
| } else { | |
| messageWebView.setContent(AwfulHtmlPage.getContainerHtml(mPrefs, null, false)); |
| if(pmId <=0){ | ||
| messageWebView.setVisibility(GONE); | ||
| }else{ | ||
| }else if (aSavedState == null) { | ||
| syncPM(); |
There was a problem hiding this comment.
Using aSavedState == null as the only gate for syncPM() can skip the network fetch even when no WebView state was actually restored (e.g., saveState returned null / state missing in the Bundle). Consider gating on “restore succeeded” (e.g., restoreState return value != null, or presence of a dedicated saved Bundle) rather than just Bundle non-nullness.
| public void onSaveInstanceState(@androidx.annotation.NonNull Bundle outState) { | ||
| super.onSaveInstanceState(outState); |
There was a problem hiding this comment.
Style consistency: elsewhere in the codebase @NonNull is imported (import androidx.annotation.NonNull) rather than using the fully-qualified @androidx.annotation.NonNull in method signatures. Importing NonNull here would match the existing convention.
Closes #691.
Reopening the PM view after popping back from the settings activity (and on configuration changes in general) shows a blank message body. The fragment uses
setRetainInstance(true)for the wrapper but the embeddedAwfulWebViewis rebuilt and its state is not preserved.This change persists the WebView in
onSaveInstanceState, restores it inonCreateView, and skips thesyncPM()refetch when there is a snapshot to restore. The blank screen goes away and the existing first-load path is unchanged.